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With experience, everyone gains ideas of good and bad practice, and that applies to both coding and code 
reviews. In her article “Five code review antipatterns,” fellow Java Champion Trisha Gee pointed out 
several worst practices for the code review process. I’d like to point out 10 antipatterns for the coding 
process itself; half are in this article, and the worst offenders are in the next article, to be published in Java 
Magazine soon. 


To be clear, you should avoid these worst practices—and eliminate them when you maintain or refactor 
existing code. And, of course, resolve them if you see these issues during a code review. 


Worst practice #10: Import messes 


The list of imported classes and methods at the top of a class is intended to be a reference to the API that 
it is using. Imports ending in * convey little specific information and, even worse, unused imports are 
misleading. Imports in a quasi-random order take longer to read, which is a pain for maintenance. 


A better way: Let your IDE maintain the imports. The Eclipse IDE has really good support for this: Its 
“organize imports” feature will, with one click, remove unused imports, add any missing imports, and sort 
the list into a consistent order, with java classes first, then javax, then third-party classes, and then 
static imports. You can get all that in IntelliJ IDEA, but you must tweak three or four settings to get there. 


True confession: When I was a young and foolish tech lead on a large app project which shall remain 
nameless, | once set the messaging level for unused imports from Warning to Error in the Eclipse 
settings and committed this to the project repository. Of course, | did this worst practice only after 
lecturing and hectoring the development team didn’t work. This was part of my plan to keep imports 
organized across the entire project. Changing this setting wasn’t popular, but the few opposing developers 
came around after seeing how easy it was to fix (using Ctr1+Shift+0) and how this change made the 
long list of imports on that project much easier to read. 


Worst practice #9: Inconsistent indentation 


The indentation-champion language is surely Python, which uses indentation instead of braces or 
keywords to denote the body of a control flow or method. Thus, if Python code is indented incorrectly it 
won't compile! 


Fortunately, Java (like the other C-family languages) uses braces for block structure and ignores 
whitespace. That said, consistent indentation still matters. While indents are not required by the compiler, 
they are required for the human reader. Consider the following code: 


aig (conci icin) 


statementl; 


statement2; 
statement3; 


Copy code snippet 


Upon a quick read, it appears as though statements 1 and 2 are controlled by the if. However, statement 
2 is not, because this is Java, not Python. 


Or consider the following code: 


statementl; 
statement2; 


statement3; 


Copy code snippet 


What was the programmer thinking? The code looks like something spewed by a waterfall on a windy day. 
The statements have the same level of control flow, so they should all begin in the same column. Again, 
modern IDEs can repair this damage in no time flat with a feature such as “Fix Indentation.” 


Select an entire file with Ctr1+A or Cmd+A, or select one method by selecting it with the mouse. Then 
choose the indentation repair from the Edit or Code menu. Problem solved! 


Worst practice #8: JAR files without links 


When Java first arrived, it appeared that it would solve one of Windows developers’ nightmares: the oft- 
cursed “DLL Hell,” where a mixture of different shared objects (such as .d11 files in Windows or . so files 
on other platforms) contain version conflicts. 


Unfortunately, the problem wasn't solved. That’s part of what the Java Platform Module System (JPMS) 
was intended to address. Tools such as Maven and Gradle have been helping with this issue for years—but 
sometimes JAR files without links still appear. 


The worst case I’ve run across is a project with a folder of files that were named something like the 
following: 


Wicd Jar 
system.jar 
financial.jar 
report.jar 


Copy code snippet 


The files had dates about 10 years old. Each of the four projects had been updated by their maintainers 
during that time, but there was no record of what version of the library JAR files was depended upon by 
the main application—unless you considered “the JAR files that happen to be in the 1ib folder” to be a 
form of documentation. 


Some of the JAR files were from third-party APIs (whose names have been changed to protect the guilty) 
that had multiple news-making security issues over the years, yet none of the developers on the team 
seemed concerned enough to move to versioned JAR files—maybe because they didn’t know if they were 
using the affected versions. 


| admit that | may have created some projects like this many years ago—but | have taken the pledge to 
avoid them. 


Today all my projects are managed by Maven or Gradle, each of which takes a specification of each 
dependency’s group (usually the organization), artifact (the JAR name), and a version number and will 
fetch the matching JAR file. That file will have the artifact name and version number in the filename. For 


example, a project might have the following in its Maven configuration file (pom. xm1): 


<dependency> 


<groupiId>com. darwinsys</groupId> 
<artifactId>darwinsys-api</artifactId> 


<version>] 


</dependency> 


lole 5£/VErSLoOn> 


Copy code snippet 


This code in pom. xml directs Maven to download the darwinsys-api-1.7.5.jar file and store it 
(along with some metadata files) in a carefully constructed tree in my home directory (which is 
~/.m2/repository/com/darwinsys/darwinsys-api/1.7.5). In this way, when two or more 
projects require the same JAR file, the JAR will be downloaded only once. 


Here is a very selective look at the Maven local repository on one of my systems. 


S ile ~/sm2/ cepost tony 
aopalliance biz bouncycastle cglib com dev eclipse edu info io 
jakarta javax jaxen jline log4j 


S ls ~/.m2/repository/com/darwinsys/darwinsys-api 
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sos 
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maven-metada 


cal-Cem 


maven-metada 


Eal-cen 


resolver-stat 


tral 


tral 


L. xml 
l.xml.shal 


tus.properties 


S ls ~/.m2/repository/com/darwinsys/darwinsys-api/1.7.5 


_ EMOTE. FEPOSİTOr LES 


carwinsys-ani=L. Vs 9 JA 


darwinsys api m7 
carwinsys-api=l. T; 5. O0 
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darwinsys-api-1.7.5.pom.shal 


$ 
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Copy code snippet 


By looking at the pom. xm1 file, not only is it clear which version of the API is used in that particular 
project, it’s also clear (at least if you know what the default is and that there is no other repository listed in 
the pom. xm1 file) that the JAR file came from the centralized Maven repository, Maven Central. 


What’s more, the JAR file itself has its version number embedded in its filename. 


Maven uses the Secure Hash Algorithm (sha) files to ensure that the JAR file hasn’t been tampered with. If 
you run the build tool in debug mode, you will see an extremely verbose output that includes the full path 
of each JAR file that is on the classpath. Plus, Maven has capabilities such as mvn dependency:tree to 
show all the sub- and sub-sub-dependencies in a tree format. 


Keeping JAR dependencies under control is part of making software development a discipline. Make it so! 


Worst practice #7: Meaningless names 
Now is a good time to quote lan’s First Rule of Coding: 


You should never type more than a few characters of any name except when you’re 
creating it. 


Given that most developers (except for two or three vi diehards) use a full-featured IDE these days, and 
since all major IDEs have really good code completion features, there’s no reason to type out long names. 


But neither is there any reason to avoid giving meaningful names to methods, fields, classes, variables, 
and other elements. 


Variable names such as i, j, and k are, in my book, allowed only when you're using the old-style for loop 
to index an array or count something. Also allowed are names such as s for a locally used String, in the 
header of a few-lines-long method, or when you're writing a lambda that is short and self-contained. 


For everything else, pick a useful name. 


This becomes particularly important where the var keyword is used to avoid having to give type 
declarations. Why? The variable name may be the only clue the reader has as to what you mean the 
variable to be used for. Consider the following example: 


oie (aime L = OF 1 < ctlincicsomDEca. lengre ar) 1 
functionData[i] = someFunction(i); 


} 


customerNames.forEach(s->s.substring(1)); // "s" OK here 


int bodyFontSize = 11; 


Copy code snippet 


lm not only talking about variables: Method names should also be meaningful. In writing JUnit tests, 
you'll find that names like test 1() and test2() and so on are not only useless: They mislead, because 
such naming implies an ordering that isn’t there. 


JUnit does not make any claim to run methods in the order in which you wrote them. Methods are, in fact, 
run in the order given by the reflection API, which is documented to return members that “are not sorted 
and are not in anv particular order ” 


“iiy pe sree Sree 


Here is an example of this antipattern. 


@Test 
piole voici cesti) 1 // Bad 


i test bere- 


And here is a better way to write it. 


@Test 


public void testPositiveResultsCorrect () 


(i TESTE WSRSs 4 6 


{ // Better 


Copy code snippet 


Copy code snippet 


Remember: You are one of the people most likely to need to read this code months or years after you 
wrote it, so be kind to yourself! 


Worst practice #6: Reinventing the flat tire 


This antipattern’s title is from a paper | worked on many years ago. “Reinventing the wheel” is a common 
English-language idiom for designing and creating something that already exists. My then-colleague 
Geoff Collyer and I took the expression one step further, in a C coding style paper Geoff and | co-wrote 
long ago in a galaxy far away. “Reinventing the flat tire” meant that a programmer not only wrote code 
whose functionality was readily available in a standard or common library, but that the new code did a 
worse job than the public API. 


Here’s an example of this antipattern. 


String[] candidates = getStrings(); 

String searchingFor = "The Lost Boys"; 

IAE Eound = =k 

ioe (ame L = 07 a < Candidates. Ibkeingiting Iar) 


LE 


(candidates [i] .equals (searchingFor)) 
found = i; 


f #7 Elat tie 


{ 


Copy code snippet 


And here is a better way. 


Arrays.sort (candidates); // start of "better" approach 
found = Arrays.binarySearch (candidates, searchingFor); 


Copy code snippet 


You might think the second approach would run more slowly, because a binary search requires the input 
be sorted. That’s true. But notice that the programmer of the antipattern forgot to break out of the loop 
when finding the match, so that code’s efficiency is terrible anyway. 


Reinventing public APIs is nothing new and is often a sign of incomplete knowledge of the API. Of course, 
it’s easy enough to make that error when languages have such a vast standard library as Java has. 


Here’s an example of reinventing an API you might or might not know; this has been in the platform since 
Java 1.7. 


var x = getValue(); // legacy way 
if (x == null) { 
x = getSomeDefaultValue(); 


ocem Owe afore sine Ikin (x4) p 


Copy code snippet 


Here’s the better, shorter way. 


var y = Objects.requireNonNullElse(getValue(), getSomeDefaultValue()); 
System.out.printin(y); 


Copy code snippet 


The first example’s programmer could have used the standard Obiects.reaquireNonNullElse( ) 


v ERT oe 


library routine, which has a variety of overloads that will help reduce coding for some common operations. 


Speaking of handling null- and not-null arguments, check out the Optional class when you need to deal 
with possibly-null values, and then quiz yourself on Optional with lambdas. 


Conclusion 


This article listed five of my favorite antipatterns. My forthcoming article, “Ten Java coding antipatterns to 
avoid: Worst practices #5 through #1,” will explore the other five. 


Dig deeper 

e Five code review antipatterns 

e Refactoring Java, Part 1: Driving agile development with test-driven development 
e Refactoring Java, Part 2: Stabilizing your legacy code and technical debt 

e Refactoring Java, Part 3: Regaining business agility by simplifying legacy code 

e How the JVM locates, loads, and runs libraries 


e Book review: Java 9 Modularity 


lan Darwin 


lan Darwin is a Java Champion who has done all kinds of development, from mainframe applications and 
desktop publishing applications for UNIX and Windows, to a desktop database application in Java, to 
healthcare apps in Java for Android. He’s the author of Java Cookbook and Android Cookbook (both from 
O'Reilly). He has also written a few courses and taught many at Learning Tree International. 
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